fix(trace): redact endpoint config errors - #91
Conversation
📝 WalkthroughEnglish
中文中文
WalkthroughTrace endpoint parsing and HTTP exporter initialization now redact sensitive URL values in configuration errors. Tests cover invalid endpoints containing credentials and API keys while preserving parse-failure context. ChangesTrace endpoint redaction
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
9c6837b to
782305b
Compare
12886ba to
960c075
Compare
* platform: add multi-tenant contracts * platform/gateway: add text loop * platform/toolpolicy: add governance bridge * platform/channeladapter: add adapter outbox skeleton * platform/gateway: enqueue outbound handoff * platform/storagerouter: add storage routing contracts * platform/gateway: add session lease * platform: add gray routing helpers * platform: add tenant budget helpers * platform: validate storage migration modes * platform: validate audit policies * platform: validate audit records * platform: add capacity estimator * platform/channeladapter: replay dead letters * platform: populate audit record ids * platform: validate audit sink writes * platform: add usage record contracts * platform: add usage sink contracts * platform: add config version contracts * platform: select config version by session gray bucket * platform: add config version lifecycle helpers * platform: add usage summary contracts * platform: add audit query contracts * platform: add config version diff contracts * platform: add config gray status summary * platform: add operational action audit contracts * platform: add config cache invalidation contracts * platform: add secret rotation status contracts * platform: add backend migration status contracts * platform: add storage router status summary * platform: add config operation summary contracts * platform/toolpolicy: add approval summary contract * fix(platform): enforce binding ACL and redact outbox errors * fix(platform): redact gateway audit error reasons * fix(toolpolicy): enforce auditable policy identity * platform: harden identity and idempotency contracts * platform: tighten routing identity contracts * fix(platform): reconcile hardened contracts with gateway * platform: add budget decision audit contracts * platform/gateway: add minimum loop acceptance test * platform/gateway: add outbound dispatch acceptance test * platform/gateway: correlate audit trace ids * platform/gateway: add trace skeleton spans * platform/gateway: enable runner session trace * platform/gateway: add message event trace contract * feat(platform): mark tool call trace spans Adds safe platform tool-call trace contract spans. Independent re-review reported P0/P1 clear; CodeRabbit only reported a trivial test-helper nitpick. * feat(platform): trace memory search spans Adds safe memory search trace spans on the current memory.Reader boundary. Independent review reported P0/P1 clear; local focused validation and build passed. * feat(platform): trace memory write spans Adds safe memory write trace spans for add/update/delete/clear. Independent review reported P0/P1 clear; local focused validation and build passed. * feat(platform): mark summary create trace spans Adds summary-create trace contract markers for Redis/PostgreSQL summary creation spans. Independent review reported P0/P1 clear; local focused validation and build passed. * fix(platform): address CI checks * fix(platform): address CodeRabbit review feedback * fix(platform): bound in-memory sink records --------- Co-authored-by: xnlemon <xianingawa@gmail.com>
960c075 to
4cc0a1a
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
telemetry/trace/trace_test.go (1)
162-193: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest the preserved error-chain contract.
The
%zzcases verify sanitized text only. Also asserterrors.As(err, &urlErr)reaches the underlying*url.Error; otherwise a future removal ofUnwrap()breaks callers without failing this regression test.中文
测试保留的错误链契约。
%zz用例目前只验证了脱敏后的错误文本。还应断言errors.As(err, &urlErr)能获取底层*url.Error;否则未来移除Unwrap()会破坏调用方,但该回归测试不会失败。As per path instructions, prioritize “error semantics” and cover “regression scenarios.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@telemetry/trace/trace_test.go` around lines 162 - 193, Extend TestStartHTTP_InvalidEndpointURLRedactsSensitiveConfig to verify the preserved error chain for the invalid endpoint cases: declare a *url.Error target and assert errors.As(err, &urlErr) succeeds, especially for the %zz inputs, while retaining the existing redaction and endpoint-context checks.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@telemetry/trace/trace_test.go`:
- Around line 162-193: Extend
TestStartHTTP_InvalidEndpointURLRedactsSensitiveConfig to verify the preserved
error chain for the invalid endpoint cases: declare a *url.Error target and
assert errors.As(err, &urlErr) succeeds, especially for the %zz inputs, while
retaining the existing redaction and endpoint-context checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50cd32fc-15e8-46ad-a3d8-86f7920429fd
📒 Files selected for processing (2)
telemetry/trace/trace.gotelemetry/trace/trace_test.go
Objective
Close a Phase2 trace configuration redaction gap by ensuring invalid telemetry endpoint errors do not expose credentials or API keys from endpoint URLs.
Changes
parseEndpointURLand the outer HTTP trace initialization error context.Error()text while preservingUnwrap()for programmatic error inspection.Validation
go test ./telemetry/tracego vet ./telemetry/tracegit diff --checkKnown Risks / Limitations
Follow-up